python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py (1)

66-175: Consider adding an empty list edge case test.

The current tests cover key scenarios well. For completeness, you might consider adding a test for vehicle_ids=[] (empty list), which should be considered unique (no duplicates in an empty set). This is a minor suggestion as the current test coverage is already solid.

🧪 Optional: Add empty list test

# Test validate_fleet_data with empty vehicle_ids (no server required)
def test_validate_fleet_data_empty_vehicle_ids():
    vehicle_locations = []

    is_valid, msg = validate_fleet_data(
        vehicle_ids=[],
        vehicle_locations=vehicle_locations,
        capacities=None,
        vehicle_time_windows=None,
        vehicle_breaks=None,
        vehicle_break_time_windows=None,
        vehicle_break_durations=None,
        vehicle_break_locations=None,
        vehicle_types=None,
        vehicle_types_dict={},
        vehicle_order_match=None,
        skip_first_trips=None,
        drop_return_trips=None,
        min_vehicles=None,
        vehicle_max_costs=None,
        vehicle_max_times=None,
        vehicle_fixed_costs=None,
    )
    assert is_valid is True
    assert msg == "Valid Fleet Data"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/cuopt_server/cuopt_server/tests/test_set_fleet_data.py` around lines
66 - 175, Add a new unit test function named
test_validate_fleet_data_empty_vehicle_ids that calls validate_fleet_data with
vehicle_ids=[] and vehicle_locations=[] (and all other parameters the same as
existing tests: capacities=None, vehicle_time_windows=None, etc.), then assert
is_valid is True and msg == "Valid Fleet Data"; this checks the empty-list edge
case and verifies the function treats an empty vehicle_ids list as valid/unique.

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py (1)

88-92: ⚠️ Potential issue | 🟠 Major

Add a deprecation phase before enforcing duplicate vehicle_ids as an error.

This introduces an immediate API behavior break. Please emit a deprecation warning first and gate hard rejection behind a config/version flag, then enforce in a later release. Downstream impact: the new tests should cover both warning mode and enforcement mode.

Proposed compatibility-safe rollout

+import os
+import warnings
...
     if vehicle_ids is not None:
         fleet_length_check_array.append(len(vehicle_ids))
         if len(vehicle_ids) != len(set(vehicle_ids)):
-            return (
-                False,
-                "vehicle_ids must be unique; duplicates are not allowed",
-            )
+            msg = "vehicle_ids must be unique; duplicates are not allowed"
+            enforce_unique_vehicle_ids = os.getenv(
+                "CUOPT_ENFORCE_UNIQUE_VEHICLE_IDS", "0"
+            ).lower() in {"1", "true", "yes"}
+            if enforce_unique_vehicle_ids:
+                return (False, msg)
+            warnings.warn(
+                msg + " (will be enforced in a future release)",
+                DeprecationWarning,
+                stacklevel=2,
+            )

As per coding guidelines, "maintain backward compatibility in Python and server APIs with deprecation warnings".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@python/cuopt_server/cuopt_server/utils/routing/validation_fleet_data.py`
around lines 88 - 92, Replace the hard-rejection on duplicate vehicle_ids with a
two-phase change: first emit a deprecation warning when duplicates are found
(use warnings.warn(..., DeprecationWarning) or your project's deprecation
helper) and only return the (False, "...") error if a config/version gate is
enabled (e.g., a flag named enforce_unique_vehicle_ids or compare a
MIN_API_VERSION); update the duplicate check around vehicle_ids in
validation_fleet_data.py to consult that gate and to log the deprecation message
when running in warning mode, and add/adjust tests to cover both warning-mode
(duplicates allowed + warning emitted) and enforcement-mode (duplicates cause
hard error).